fix(castore): introduce fast path while getting torrent metadata to reduce allocs by sambhav-jain-16 · Pull Request #611 · uber/kraken

sambhav-jain-16 · 2026-05-02T15:32:36Z

What?

TorrentMeta.Serialize/Deserialize round-trip svia JSON (json.Marshal + json.Unmarshal) for a MetaInfo. For every blob this allocates a fresh []byte buffer and a new *core.MetaInfo on every call to GetCacheFileMetadata. The fast path short-circuits it when the entry is already in the memory cache it hands back the existing *core.MetaInfo pointer directly, so a metadata read becomes a single pointer assignment with zero heap allocations.

Why only TorrentMetadata ?

TorrentMeta is the only metadata type whose backing value (*core.MetaInfo) is immutable after creation. Other metadata types (e.g. LastAccessTime) are mutable values written on every access.

Testing

Added a benchmark BenchmarkGetCacheFileMetadata_MemoryCache with different blob sizes and metadata sizes
Following is the benchstat comparison.

goos: linux
goarch: amd64
pkg: github.com/uber/kraken/lib/store
cpu: AMD EPYC 7B13
                                                       │ /home/user/kraken/bench-results/run-20260502-161433-n50/before.txt │ /home/user/kraken/bench-results/run-20260502-161433-n50/after.txt │
                                                       │                               sec/op                               │                  sec/op                    vs base                │
GetCacheFileMetadata_MemoryCache/1MB_4pc-96                                                                   7503.00n ± 1%                                 57.97n ± 1%  -99.23% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_64pc-96                                                                27221.50n ± 1%                                 56.61n ± 1%  -99.79% (n=50)
GetCacheFileMetadata_MemoryCache/64MB_256pc-96                                                               89182.50n ± 1%                                 57.67n ± 1%  -99.94% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_4pc_4MBpc-96                                                            7256.50n ± 1%                                 56.99n ± 3%  -99.21% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_16pc_1MBpc-96                                                          11599.50n ± 1%                                 61.23n ± 2%  -99.47% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_1024pc_16KBpc-96                                                      334380.00n ± 2%                                 61.69n ± 4%  -99.98% (p=0.000 n=50)
geomean                                                                                                         28.29µ                                      58.66n       -99.79%

                                                       │ /home/user/kraken/bench-results/run-20260502-161433-n50/before.txt │ /home/user/kraken/bench-results/run-20260502-161433-n50/after.txt │
                                                       │                                B/op                                │                       B/op                         vs base        │
GetCacheFileMetadata_MemoryCache/1MB_4pc-96                                                                   1907.000 ± 0%                                          8.000 ± 0%  -99.58% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_64pc-96                                                                 5103.000 ± 0%                                          8.000 ± 0%  -99.84% (n=50)
GetCacheFileMetadata_MemoryCache/64MB_256pc-96                                                               16494.500 ± 0%                                          8.000 ± 0%  -99.95% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_4pc_4MBpc-96                                                            1898.000 ± 0%                                          8.000 ± 0%  -99.58% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_16pc_1MBpc-96                                                           2732.000 ± 0%                                          8.000 ± 0%  -99.71% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_1024pc_16KBpc-96                                                       67460.500 ± 0%                                          8.000 ± 0%  -99.99% (n=50)
geomean                                                                                                        6.043Ki                                               8.000       -99.87%

                                                       │ /home/user/kraken/bench-results/run-20260502-161433-n50/before.txt │ /home/user/kraken/bench-results/run-20260502-161433-n50/after.txt │
                                                       │                             allocs/op                              │                     allocs/op                      vs base        │
GetCacheFileMetadata_MemoryCache/1MB_4pc-96                                                                     37.000 ± 0%                                          1.000 ± 0%  -97.30% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_64pc-96                                                                  103.000 ± 0%                                          1.000 ± 0%  -99.03% (n=50)
GetCacheFileMetadata_MemoryCache/64MB_256pc-96                                                                 299.000 ± 0%                                          1.000 ± 0%  -99.67% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_4pc_4MBpc-96                                                              37.000 ± 0%                                          1.000 ± 0%  -97.30% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_16pc_1MBpc-96                                                             52.000 ± 0%                                          1.000 ± 0%  -98.08% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_1024pc_16KBpc-96                                                        1072.000 ± 0%                                          1.000 ± 0%  -99.91% (n=50)
geomean                                                                                                          115.3                                               1.000       -99.13%

Copilot

Pull request overview

Improves CAStore.GetCacheFileMetadata performance when serving torrent metadata from the in-memory cache by avoiding per-read serialization/deserialization, and adds coverage/benchmarks to validate allocation behavior.

Changes:

Add a fast path in CAStore.GetCacheFileMetadata to return the cached *core.MetaInfo pointer directly for *metadata.TorrentMeta.
Add a unit test asserting the no-copy behavior when reading metadata from the memory cache.
Add a benchmark suite to measure allocations/time across different blob sizes and piece counts for memory-cache metadata reads.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`lib/store/ca_store.go`	Returns cached `*core.MetaInfo` directly for torrent metadata reads from memory cache, avoiding JSON round-trips.
`lib/store/ca_store_test.go`	Adds a no-copy unit test and a benchmark measuring metadata read allocations across scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

thijmv

A couple of comments:

thijmv · 2026-05-04T10:23:04Z

Do we want to remove the serialisation path until other metadata types are added and return an error instead? In the current implementation, this path would yield a silent invariant violation, as torrent metadata is currently the only supported type.

I want to be as defensive as possible with this change to avoid any regressions. Since this is the current production behavior, let's keep it this way, if you feel strongly against it, we can discuss more.

This function is also called for metadata.Persist here

kraken/origin/blobserver/server.go

Lines 1026 to 1029 in f862dd5

var pm metadata.Persist

if err := s.cas.GetCacheFileMetadata(name, &pm); err != nil && !os.IsNotExist(err) {

return false, fmt.Errorf("store: %s", err)

}

It's better we keep it this way to be cleaner

Since this PR aims to remove the need for serialisation by returning a metadata pointer instead, would it not make more sense to either completely remove it or at least log an error as we expect that path to never be reached?

When we call it using metadata.Persist, the metadata is expected to be fetched from disk. Could you elaborate on the relevance?

@sambhav-jain-16 Do I understand correctly that ALWAYS expect the if check to be true? If so, can we emit an error log in the else case, so we catch a potential invariant violation if it happens?

@thijmv @Anton-Kalpakchiev
I took another look, and I agree with your suggestions.

When we call it using metadata.Persist, the metadata is expected to be fetched from disk. Could you elaborate on the relevance?

I agree on this, given that as of now we are not using the in-mem cache unto upload path, this case shouldn't happen.

I'll make the changes and update

@Anton-Kalpakchiev @thijmv
Made the changes. PTAL

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

+			// hand back cached pointer
+			tm.MetaInfo = entry.MetaInfo
+			return nil


+			// Verify if the asked metadata is TorrentMetdata or not
+			tm, ok := md.(*metadata.TorrentMeta)
+			if !ok {
+				return fmt.Errorf("unvariant violation: GetCacheFileMetadata should only be called for TorrentMetadata")


Copilot AI review requested due to automatic review settings May 2, 2026 15:32

Copilot started reviewing on behalf of sambhav-jain-16 May 2, 2026 15:33 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

Comment thread lib/store/ca_store_test.go

sambhav-jain-16 requested a review from Copilot May 2, 2026 16:25

Copilot started reviewing on behalf of sambhav-jain-16 May 2, 2026 16:25 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

Comment thread lib/store/ca_store_test.go

sambhav-jain-16 force-pushed the torrent-metadata-alloc branch from b1e3e09 to 42420f8 Compare May 2, 2026 16:31

sambhav-jain-16 requested a review from Copilot May 2, 2026 16:31

Copilot started reviewing on behalf of sambhav-jain-16 May 2, 2026 16:32 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

sambhav-jain-16 changed the title ~~Torrent metadata alloc~~ fix(castore): introduce fast patch while getting torrent metadata to reduce allocs May 3, 2026

sambhav-jain-16 marked this pull request as ready for review May 3, 2026 10:23

sambhav-jain-16 changed the title ~~fix(castore): introduce fast patch while getting torrent metadata to reduce allocs~~ fix(castore): introduce fast path while getting torrent metadata to reduce allocs May 3, 2026

thijmv reviewed May 4, 2026

View reviewed changes

sambhav-jain-16 added 4 commits May 8, 2026 17:23

add benchmark and tests for GetMetdataFromCache

db10fce

add fast path to avoid data copy for torrentmetadata

c089d36

add defer for stop timer

106f764

use b.Loop()

f7f0aeb

Copilot AI review requested due to automatic review settings May 8, 2026 17:24

sambhav-jain-16 force-pushed the torrent-metadata-alloc branch from 6aaaf7d to 33637e0 Compare May 8, 2026 17:24

add invariant violation for other metadata types

d4a51c1

Copilot started reviewing on behalf of sambhav-jain-16 May 8, 2026 17:24 View session

sambhav-jain-16 force-pushed the torrent-metadata-alloc branch from 33637e0 to d4a51c1 Compare May 8, 2026 17:25

Copilot AI reviewed May 8, 2026

View reviewed changes

	var pm metadata.Persist
	if err := s.cas.GetCacheFileMetadata(name, &pm); err != nil && !os.IsNotExist(err) {
	return false, fmt.Errorf("store: %s", err)
	}

Conversation

sambhav-jain-16 commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why only TorrentMetadata ?

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

thijmv left a comment

Choose a reason for hiding this comment

Uh oh!

thijmv May 4, 2026

Choose a reason for hiding this comment

Uh oh!

sambhav-jain-16 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

sambhav-jain-16 May 5, 2026

Choose a reason for hiding this comment

Uh oh!

thijmv May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Anton-Kalpakchiev May 6, 2026

Choose a reason for hiding this comment

Uh oh!

sambhav-jain-16 May 6, 2026

Choose a reason for hiding this comment

Uh oh!

sambhav-jain-16 May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sambhav-jain-16 commented May 2, 2026 •

edited

Loading